Upstream some more Git for Windows' patches - #2195
Open
dscho wants to merge 12 commits into
Open
Conversation
While Git for Windows does not _ship_ Python (in order to save on bandwidth), MSYS2 provides very fine Python interpreters that users can easily take advantage of, by using Git for Windows within its SDK. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is no longer true in general, not with supporting Clang out of the box. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This option was added in fa93bb2 (MinGW: Fix stat definitions to work with MinGW runtime version 4.0, 2013-09-11), i.e. a _long_ time ago. So long, in fact, that it still targeted MinGW. But we switched to mingw-w64 in 2015, which seems not to share the problem, and therefore does not require a fix. Even worse: This flag is incompatible with UCRT64, which we are about to support by way of upstreaming `mingw-w64-git` to the MSYS2 project, see msys2/MINGW-packages#26470 for details. So let's send that option into its well-deserved retirement. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
That option only matters there, and is in fact only really understood in those builds; UCRT64 versions of GCC, for example, do not know what to do with that option. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In bf2d5d8 (Don't let ld strip relocations, 2016-01-16) (picked from git-for-windows@6a237925bf10), Git for Windows introduced the `-Wl,-pic-executable` flag, specifying the exact entry point via `-e`. This required discerning between i686 and x86_64 code because the former required the symbol to be prefixed with an underscore, the latter did not. As per https://sourceware.org/bugzilla/show_bug.cgi?id=10865, the specified symbols are already the default, though. So let's drop the overly-specific definition. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
MSYS2 already defines a couple of helpful environment variables, and we can use those to infer the installation location as well as the CPU. No need for hard-coding ;-) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The tell-tale is the presence of the `MSYSTEM` value while compiling, of course. In that case, we want to ensure that `MSYSTEM` is set when running `git.exe`, and also enable the magic MSYS2 tty detection. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Member
Author
|
/submit |
|
Submitted as pull.2195.git.1785939999.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -524,6 +518,7 @@ ifeq ($(uname_S),Windows) | |||
| NO_POSIX_GOODIES = UnfortunatelyYes | |||
There was a problem hiding this comment.
Junio C Hamano wrote on the Git mailing list (how to reply to this email):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> The "dashed form" has been officially deprecated in Git version 1.5.4,
> which was released on February 2nd, 2008, i.e. a very long time ago.
> This deprecation was never finalized by skipping these hard-links, but
> we can start the process now, in Git for Windows.
Good. Perhaps somebody (you do not have to volunteer) can champion
their removal from everywhere at Git 3.0 version boundary?
Thanks.
| @@ -465,14 +465,8 @@ ifeq ($(uname_S),Windows) | |||
| GIT_VERSION := $(GIT_VERSION).MSVC | |||
There was a problem hiding this comment.
Junio C Hamano wrote on the Git mailing list (how to reply to this email):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> - ifeq (CLANGARM64,$(MSYSTEM))
> - prefix = /clangarm64
> - else
> - prefix = /mingw64
> - endif
> + ifneq (,$(MSYSTEM))
> + prefix = $(MINGW_PREFIX)
> endif
Mental note: if $(MSYSTEM) is not an empty string, we set prefix to
$(MINGW_PREFIX).
> @@ -755,6 +749,10 @@ ifeq ($(uname_S),MINGW)
> BASIC_LDFLAGS += -Wl,--dynamicbase
> endif
> ifneq (,$(MSYSTEM))
> + ifeq ($(MINGW_PREFIX),$(filter-out /%,$(MINGW_PREFIX)))
> + # Override if empty or does not start with a slash
> + MINGW_PREFIX := /$(shell echo '$(MSYSTEM)' | tr A-Z a-z)
> + endif
Mental note: MINGW_PREFIX that does not begin with a slash is forced
to begin with a slash.
> prefix = $(MINGW_PREFIX)
And that becomes $(prefix).
> diff --git a/meson.build b/meson.build
> index 7073d5844d..6ddc461873 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1318,7 +1318,6 @@ elif host_machine.system() == 'windows'
>
> libgit_c_args += [
> '-DDETECT_MSYS_TTY',
> - '-DENSURE_MSYSTEM_IS_SET',
> '-DNATIVE_CRLF',
> '-DNOGDI',
> '-DNO_POSIX_GOODIES',
> @@ -1328,6 +1327,18 @@ elif host_machine.system() == 'windows'
> '-D__USE_MINGW_ANSI_STDIO=0',
> ]
>
> + msystem = get_option('msystem')
> + if msystem != ''
> + mingw_prefix = get_option('mingw_prefix')
> + if mingw_prefix == ''
> + mingw_prefix = '/' + msystem.to_lower()
> + endif
> + libgit_c_args += [
> + '-DENSURE_MSYSTEM_IS_SET="' + msystem + '"',
> + '-DMINGW_PREFIX="' + mingw_prefix + '"'
> + ]
> + endif
Lowercase mingw_prefix in Meson world corresponds to MINGW_PREFIX in
Make world, I guess. -DMINGW_PRFIX gets mingw_prefix which begins
with a slash.
I do not do Windows or Meson, but doesn't this contradict with what
we have in [12/12], part of which says:
diff --git a/config.mak.uname b/config.mak.uname
index 2f7d445eb3..0b63be10b7 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -535,7 +535,9 @@ endif
compat/win32/pthread.o compat/win32/syslog.o \
compat/win32/trace2_win32_process_info.o \
compat/win32/dirent.o
- COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
+ COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY \
+ -DENSURE_MSYSTEM_IS_SET="\"$(MSYSTEM)\"" -DMINGW_PREFIX="\"$(patsubst /%,%,$(MINGW_PREFIX))\"" \
+ -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -ENTRY:wmainCRTStartup -SUBSYSTEM:CONSOLE
# invalidcontinue.obj allows Git's source code to close the same file
# handle twice, or to access the osfhandle of an already-closed stdout
IOW, -DMINGW_PREFIX passed to the compiler strips leading slash from
$(MINGW_PREFIX).
Isn't it necessary to strip the leading slash from ming_prefix also
on the Meson side?There was a problem hiding this comment.
Johannes Schindelin wrote on the Git mailing list (how to reply to this email):
Hi Junio,
On Wed, 5 Aug 2026, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> [...]
> > diff --git a/meson.build b/meson.build
> > index 7073d5844d..6ddc461873 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1318,7 +1318,6 @@ elif host_machine.system() == 'windows'
> >
> > libgit_c_args += [
> > '-DDETECT_MSYS_TTY',
> > - '-DENSURE_MSYSTEM_IS_SET',
> > '-DNATIVE_CRLF',
> > '-DNOGDI',
> > '-DNO_POSIX_GOODIES',
> > @@ -1328,6 +1327,18 @@ elif host_machine.system() == 'windows'
> > '-D__USE_MINGW_ANSI_STDIO=0',
> > ]
> >
> > + msystem = get_option('msystem')
> > + if msystem != ''
> > + mingw_prefix = get_option('mingw_prefix')
> > + if mingw_prefix == ''
> > + mingw_prefix = '/' + msystem.to_lower()
> > + endif
> > + libgit_c_args += [
> > + '-DENSURE_MSYSTEM_IS_SET="' + msystem + '"',
> > + '-DMINGW_PREFIX="' + mingw_prefix + '"'
> > + ]
> > + endif
>
> Lowercase mingw_prefix in Meson world corresponds to MINGW_PREFIX in
> Make world, I guess. -DMINGW_PRFIX gets mingw_prefix which begins
> with a slash.
>
> I do not do Windows or Meson, but doesn't this contradict with what
> we have in [12/12], part of which says:
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 2f7d445eb3..0b63be10b7 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -535,7 +535,9 @@ endif
> compat/win32/pthread.o compat/win32/syslog.o \
> compat/win32/trace2_win32_process_info.o \
> compat/win32/dirent.o
> - COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
> + COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY \
> + -DENSURE_MSYSTEM_IS_SET="\"$(MSYSTEM)\"" -DMINGW_PREFIX="\"$(patsubst /%,%,$(MINGW_PREFIX))\"" \
> + -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
> BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -ENTRY:wmainCRTStartup -SUBSYSTEM:CONSOLE
> # invalidcontinue.obj allows Git's source code to close the same file
> # handle twice, or to access the osfhandle of an already-closed stdout
>
>
> IOW, -DMINGW_PREFIX passed to the compiler strips leading slash from
> $(MINGW_PREFIX).
>
> Isn't it necessary to strip the leading slash from ming_prefix also
> on the Meson side?
Correct. Since Git for Windows does not use Meson to build the project,
and since the Visual C-based builds (including CI's `windows-meson-build`)
do not use `MSYSTEM`, this was not caught earlier. The next iteration will
have the fix.
Ciao,
JohannesMSYS2 defines some helpful environment variables, e.g. `MSYSTEM`. There is code in Git for Windows to ensure that that `MSYSTEM` variable is set, hard-coding a default. However, the existing solution jumps through hoops to reconstruct the proper default, and is even incomplete doing so, as we found out when we extended it to support CLANGARM64. This is absolutely unnecessary because there is already a perfectly valid `MSYSTEM` value we can use at build time. This is even true when building the MINGW32 variant on a MINGW64 system because `makepkg-mingw` will override the `MSYSTEM` value as per the `MINGW_ARCH` array. The same is equally true for the `/mingw64`, `/mingw32` and `/clangarm64` prefix: those values are already available via the `MINGW_PREFIX` environment variable, and we just need to pass that setting through. Only when `MINGW_PREFIX` is not set (as is the case in Git for Windows' minimal SDK, where only `MSYSTEM` is guaranteed to be set correctly), we use as fall-back the top-level directory whose name is the down-cased value of the `MSYSTEM` variable. Incidentally, this also broadens the support to all the configurations supported by the MSYS2 project, i.e. clang64 & ucrt64, too. Note: This keeps the same, hard-coded MSYSTEM platform support for CMake as before, but drops it for Meson (because it is unclear how Meson could do this in a more flexible manner). Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It is merely a historical wart that, say, `git-commit` exists in the `libexec/git-core/` directory, a tribute to the original idea to let Git be essentially a bunch of Unix shell scripts revolving around very few "plumbing" (AKA low-level) commands. Git has evolved a lot from there. These days, most of Git's functionality is contained within the `git` executable, in the form of "built-in" commands. To accommodate for scripts that use the "dashed" form of Git commands, even today, Git provides hard-links that make the `git` executable available as, say, `git-commit`, just in case that an old script has not been updated to invoke `git commit`. Those hard-links do not come cheap: they take about half a minute for every build of Git on Windows, they are mistaken for taking up huge amounts of space by some Windows Explorer versions that do not understand hard-links, and therefore many a "bug" report had to be addressed. The "dashed form" has been officially deprecated in Git version 1.5.4, which was released on February 2nd, 2008, i.e. a very long time ago. This deprecation was never finalized by skipping these hard-links, but we can start the process now, in Git for Windows. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Special-casing even more configurations simply does not make sense. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
A change between versions 2.4.1 and 2.6.0 of the MSYS2 runtime modified how Cygwin's runtime (and hence Git for Windows' MSYS2 runtime derivative) handles locales: d16a56306d (Consolidate wctomb/mbtowc calls for POSIX-1.2008, 2016-07-20). An unintended side-effect is that "cold-calling" into the POSIX emulation will start with a locale based on the current code page, something that Git for Windows is very ill-prepared for, as it expects to be able to pass a command-line containing non-ASCII characters to the shell without having those characters munged. One symptom of this behavior: when `git clone` or `git fetch` shell out to call `git-upload-pack` with a path that contains non-ASCII characters, the shell tried to interpret the entire command-line (including command-line parameters) as executable path, which obviously must fail. This fixes git-for-windows#1036 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows wants to add `git.exe` to the users' `PATH`, without cluttering the latter with unnecessary executables such as `wish.exe`. To that end, it invented the concept of its "Git wrapper", i.e. a tiny executable located in `C:\Program Files\Git\cmd\git.exe` (originally a CMD script) whose sole purpose is to set up a couple of environment variables and then spawn the _actual_ `git.exe` (which nowadays lives in `C:\Program Files\Git\mingw64\bin\git.exe` for 64-bit, and the obvious equivalent for 32-bit installations). Currently, the following environment variables are set unless already initialized: - `MSYSTEM`, to make sure that the MSYS2 Bash and the MSYS2 Perl interpreter behave as expected, and - `PLINK_PROTOCOL`, to force PuTTY's `plink.exe` to use the SSH protocol instead of Telnet, - `PATH`, to make sure that the `bin` folder in the user's home directory, as well as the `/mingw64/bin` and the `/usr/bin` directories are included. The trick here is that the `/mingw64/bin/` and `/usr/bin/` directories are relative to the top-level installation directory of Git for Windows (which the included Bash interprets as `/`, i.e. as the MSYS pseudo root directory). Using the absence of `MSYSTEM` as a tell-tale, we can detect in `git.exe` whether these environment variables have been initialized properly. Therefore we can call `C:\Program Files\Git\mingw64\bin\git` in-place after this change, without having to call Git through the Git wrapper. Obviously, above-mentioned directories must be _prepended_ to the `PATH` variable, otherwise we risk picking up executables from unrelated Git installations. We do that by constructing the new `PATH` value from scratch, appending `$HOME/bin` (if `HOME` is set), then the MSYS2 system directories, and then appending the original `PATH`. Side note: this modification of the `PATH` variable is independent of the modification necessary to reach the executables and scripts in `/mingw64/libexec/git-core/`, i.e. the `GIT_EXEC_PATH`. That modification is still performed by Git, elsewhere, long after making the changes described above. While we _still_ cannot simply hard-link `mingw64\bin\git.exe` to `cmd` (because the former depends on a couple of `.dll` files that are only in `mingw64\bin`, i.e. calling `...\cmd\git.exe` would fail to load due to missing dependencies), at least we can now avoid that extra process of running the Git wrapper (which then has to wait for the spawned `git.exe` to finish) by calling `...\mingw64\bin\git.exe` directly, via its absolute path. Testing this is in Git's test suite tricky: we set up a "new" MSYS pseudo-root and copy the `git.exe` file into the appropriate location, then verify that `MSYSTEM` is set properly, and also that the `PATH` is modified so that scripts can be found in `$HOME/bin`, `/mingw64/bin/` and `/usr/bin/`. This addresses git-for-windows#2283 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
upstream-some-more-git-for-windows-patches
branch
from
August 6, 2026 08:43
f822133 to
679d1fd
Compare
|
This branch is now known as |
|
This patch series was integrated into seen via git@8b87133. |
| @@ -3134,6 +3134,45 @@ int xwcstoutf(char *utf, const wchar_t *wcs, size_t utflen) | |||
| return -1; | |||
There was a problem hiding this comment.
Junio C Hamano wrote on the Git mailing list (how to reply to this email):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> @@ -3186,6 +3225,32 @@ static void setup_windows_environment(void)
> setenv("HOME", tmp, 1);
> }
>
> + if (!getenv("PLINK_PROTOCOL"))
> + setenv("PLINK_PROTOCOL", "ssh", 0);
> +
> +#ifdef ENSURE_MSYSTEM_IS_SET
> + if (!(tmp = getenv("MSYSTEM")) || !tmp[0]) {
Checking tmp[0] is a sign that we do not consider MSYSTEM set to an
empty string a sane state and ENSURE_MSYSTEM_IS_SET is about
correcting it, right?
> + const char *home = getenv("HOME"), *path = getenv("PATH");
> + char buf[32768];
> + size_t off = 0;
> +
> + setenv("MSYSTEM", ENSURE_MSYSTEM_IS_SET, 1);
In config.mak.uname, ENSURE_MSYSTEM_IS_SET is defined to "$(MSYSTEM)".
+ COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY \
+ -DENSURE_MSYSTEM_IS_SET="\"$(MSYSTEM)\"" -DMINGW_PREFIX="\"$(patsubst /%,%,$(MINGW_PREFIX))\"" \
Can $(MSYSTEM) be an empty string or undefined at the build time,
making ENSURE_MSYSTEM_IS_SET set to "" (two double-quotes)? Which
would mean we are exporting MSYSTEM defined to be an empty string as
well with this setenv.
It seems ifeq($(uname_S),MINGW) side protects against this situation
by placing the cflags definition
+ COMPAT_CFLAGS += -DDETECT_MSYS_TTY \
+ -DENSURE_MSYSTEM_IS_SET="\"$(MSYSTEM)\"" \
inside "ifneq (,$(MSYSTEM))..endif". That way, ENSURE_MSYSTEM_IS_SET
is not defined to "" (two double-quotes), so #ifdef ENSURE_MSYSTEM_IS_SET
would not kick in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When rebasing a Git for Windows commit to enable Rust in Windows CI, I noticed just how many of the preceding commits touching adjacent code haven't been upstreamed yet. This patch series rectifies that.
Changes since v1:
meson-side handling of theMINGW_PREFIXconstant (this was not noticed in the Git for Windows project because it does not use Meson to build the project).